Thread: Reversing a string declared as char[] works, but as char* it segfaults. Why?

Code:

if (0 == fstatfs(*to_fd, &sfs) && ('a' == *(a = sfs.f_fstypename) && 'c' == *++a && 'f' == *++a && 's' == *++a && 0 == *++a)) {

This is the type of stuff that is very wrong.

Originally Posted by kodax

Use return to exit early of a void function.

Code:

void abcd() {
    if (done) {
        return; 
    }
    // do something
}

Return statement is optional in that context, but it is necessary in x86-64 Assembly.

What does this have to do with the topic? And what does x86-64 assembly have to do with C? A C compiler will generate correct code for whatever target architecture it's compiling for (assuming the C code itself is correct, i.e., does not invoke undefined behavior). Falling off the end of a function that returns void (like abcd in your example) without a return statement is perfectly valid regardless of the target architecture.

Code:

void copy1(char *arg1, const char *arg2) {
    while ('\0' != (*arg1++ = *arg2++));
    return;
}

void copy2(char *arg1, const char *arg2) {
    unsigned long i = 0;
    while ('\0' != arg2[i]) {
        arg1[i] = arg2[i];
        i = 1 + i;
    }
    return;
}

Destructive Mutation.

You may not realize it,
but you are more inclined to use an argument as a variable,
and modify it * arg2 ++, which is the case in copy1().

This never happen in copy2().

Originally Posted by kodax

Code:

if (0 == fstatfs(*to_fd, &sfs) && ('a' == *(a = sfs.f_fstypename) && 'c' == *++a && 'f' == *++a && 's' == *++a && 0 == *++a)) {

This is the type of stuff that is very wrong.

Is it "wrong" only stylistically (which is subjective) or is it "wrong" as in incorrect? I ask because that code is not incorrect. It does what it's supposed to do (get the file system type and check that it's "acfs") and does not have undefined behavior. The logical and operator (&&) introduces a sequence point, if you're implying it has undefined behavior because a is modified multiple times.

Stylistically, I would have used strcmp() to compare the file system type with "acfs" (and swap the operands of ==, because speak like Yoda I do not), like this:

Code:

    if (fstatfs(*to_fd, &sfs) == 0 &&
        strcmp(sfs.f_fstypename, "acfs") == 0) {

but perhaps the authors of this code chose not to use strcmp() for performance or some other reasons.

I wrote that back then, and now I have to change the whole thing.

Originally Posted by kodax

Code:

void copy1(char *arg1, const char *arg2) {
    while ('\0' != (*arg1++ = *arg2++));
    return;
}

void copy2(char *arg1, const char *arg2) {
    unsigned long i = 0;
    while ('\0' != arg2[i]) {
        arg1[i] = arg2[i];
        i = 1 + i;
    }
    return;
}

Destructive Mutation.

You may not realize it,
but you are more inclined to use an argument as a variable,
and modify it * arg2 ++, which is the case in copy1().

This never happen in copy2().

Sigh, I feel like I'm playing whack-a-mole.

What is wrong with modifying an argument, especially in such a simple function?

The copy1() function is still correct, more correct than copy2(), in fact, because unsigned long isn't necessarily big enough to index the full range of the source string--imagine unsigned long is only 32 bits wide (which it is on 64-bit Windows) but the string being copied is 5 GB; whoops! i wraps around to 0 after 4 GB (before it reaches the null terminator) and your program has an infinite loop.

(And the return statement at the end of each function is unnecessary and superfluous as I already mentioned in #32.)

Originally Posted by kodax

I wrote that back then, and now I have to change the whole thing.

You wrote the file system code "back then"? Why do you have to change the whole thing? If it works, why change it? Except maybe to use strcmp() as I suggested.

Let me summarize: you're afraid to properly understand pointer arithmetic (especially post/pre-increment/decrement), so you instead choose to write buggy code that uses undersized array indices.

Originally Posted by kodax

Code:

if (0 == fstatfs(*to_fd, &sfs) && ('a' == *(a = sfs.f_fstypename) && 'c' == *++a && 'f' == *++a && 's' == *++a && 0 == *++a)) {

This is the type of stuff that is very wrong.

Yes, I agree. Yoda conditionals are so very wrong...

Originally Posted by kodax

[CODE]
Destructive Mutation.

You may not realize it,
but you are more inclined to use an argument as a variable,
and modify it * arg2 ++, which is the case in copy1().

This never happen in copy2().

Because I've never heard of it, I did a quick google for '"destructive mutation" programming'. There were five hits for C# , and they were "non-destructive mutation".

At this point you are making things up and spouting them as gospel.

https://media1.giphy.com/media/l3q2F...giphy.gif&ct=g

Why do you have to change the whole thing?

Here is a real-life scenario, a one dimensional java-guy happens to
skim through a "poorly" documented C-project.

Somehow all your projects use pointer arithmetic,
now your project will look foreign to him,
in that sense, you have unknowingly discredited all your work.

Why do you have to change the whole thing?

Pointer arithmetic is ugly.

I say 2 + 2 is 4, you say 2 + 2 is 5,
Then there is nothing that I can do for you.

Sigh, I feel like I'm playing whack-a-mole.

Then continue playing dumb.
You know that you are playing dumb, this means that you are a phony,
and everyone can see it.